-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[When] Add support for flatten_all_tuples=False #1267
Conversation
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1267 +/- ##
==========================================
+ Coverage 85.94% 85.96% +0.01%
==========================================
Files 169 169
Lines 17727 17765 +38
==========================================
+ Hits 15236 15272 +36
- Misses 2491 2493 +2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rsetaluri I needed this change to get xmr working with unflattened tuples:
The assert is no longer valid because the paths are flattened but the module outputs aren't (we could compare to the flattened results length).
The path separator changes because the tuples values aren't flattened anymore (need to do a struct reference).
Does this make sense or am I misunderstanding what's going on? I will update the path separator logic to based on the flatten_all_tuples option
The path string should use .
instead of _
since the fields aren't flattened.
^ this comment is in reference to c6d802d |
Updated inline_verilog2 test to check both cases, skipped inline_verilog since we're deprecating that anyways |
# Change this to validate mlir->verilog compile doesn't error | ||
_OUTPUT_TYPE = "mlir" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what the purpose of this is? we should either be testing mlir or mlir-verilog. if we need to test the latter then it should just be that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use it locally to ensure the compilation doesn't fail, this allows me to change the output for the entire file with a single variable, but we leave it on mlir for CI for speed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's make it into a structured environment variable then
Fixes #1316 Ensures that recursive types check tuple key ordering
* [Bind2] Separate return statements * [Bind2] Remove compile_guard argument * [CompileGuard] Store compile guard info directly * [CompileGuard] Expose active compile guards * [Bind2] Stash active compile guards Attaches active compile guard info's to bound instance metadata. * [Bind2] Add tests for compile guarded bind2() * [MLIR] Print attr_dict for sv.IfDefOp * [MLIR] Add MlirOp.parent_op() function * [MLIR] Add support for bind2 wrapped in compile guard * Emit sv.IfDefOp around sv.BindOp * Attach hw.OutputFileAttr for entire sub-tree of op's surrounding sv.BindOp (all enclosing sv.IfDefOp, and including sv.BindOp). * Add generic MlirOpPass infrastructure * [CompileGuard] Fix test * [MLIR] Update golds * [MLIR] Use := in MlirOp.parent_op() * [MLIR] Make MlirOpPass callable interface * [CompileGuard] Use Stack iterable interface * [Bind2] Use getattr to get bound inst info
Fixes #1269 Adds a pass that splits when blocks with a cyclic dependency.
This reverts commit 512c7e1.
Fixes #1253